-
Notifications
You must be signed in to change notification settings - Fork 820
Add cli_args/CLIArgs to Python and Go SDKs for cross-SDK consistency #394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: friggeri <106686+friggeri@users.noreply.github.com>
Co-authored-by: friggeri <106686+friggeri@users.noreply.github.com>
Co-authored-by: friggeri <106686+friggeri@users.noreply.github.com>
Co-authored-by: friggeri <106686+friggeri@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a cross-SDK “pass extra Copilot CLI flags” option to the Python and Go SDKs, aligning them with existing Node.js and .NET capabilities when spawning the Copilot CLI JSON-RPC server.
Changes:
- Python: introduce
cli_args: list[str]inCopilotClientOptions, defensively copied and prepended to SDK-managed CLI flags when starting the CLI process. - Go: introduce
CLIArgs []stringinClientOptions, defensively copied and prepended to SDK-managed CLI flags when starting the CLI process. - Docs + unit tests added/updated for both SDKs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| python/copilot/types.py | Adds cli_args option to Python client options type. |
| python/copilot/client.py | Copies cli_args into stored options and prepends them when spawning the CLI server. |
| python/test_client.py | Adds unit tests validating default, acceptance, and defensive copying of cli_args. |
| python/README.md | Documents cli_args option. |
| go/types.go | Adds CLIArgs to Go ClientOptions. |
| go/client.go | Defensively copies CLIArgs into options and prepends them when spawning the CLI server. |
| go/client_test.go | Adds unit tests for default behavior, acceptance, empty slice handling, and defensive copying. |
| go/README.md | Documents CLIArgs option. |
✅ Cross-SDK Consistency ReviewI've reviewed this PR for cross-language SDK consistency. Overall, this is an excellent consistency improvement that successfully brings Python and Go SDKs to feature parity with Node.js and .NET. ✅ What's Great
📊 Minor Observation: Test Coverage DisparityThis PR actually improves the overall consistency of the repository by adding unit tests for the new implementations. However, there's now a slight testing inconsistency:
This is not a blocker for this PR since you're not responsible for the Node.js/.NET implementations, but it's worth noting for future work that Node.js and .NET could benefit from similar unit test coverage. ✅ RecommendationApprove and merge. This PR successfully resolves the SDK inconsistency reported in issue #381 and follows all cross-language consistency best practices.
|
Python and Go SDKs lacked the CLI arguments pass-through option available in Node.js and .NET, preventing users from passing custom flags to the underlying Copilot CLI process.
Changes
Python SDK (
cli_args: list[str])CopilotClientOptionsTypedDict_start_cli_serverlist()to prevent mutationsGo SDK (
CLIArgs []string)ClientOptionsstructstartCLIServerappend([]string{}, ...)patternDocumentation
Usage
User-provided arguments are inserted before SDK-managed flags like
--headlessand--no-auto-update, allowing CLI customization while preserving SDK functionality.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/copilot_internal/user/opt/hostedtoolcache/node/22.22.0/x64/bin/node node /home/REDACTED/work/copilot-sdk/copilot-sdk/nodejs/node_modules/@github/copilot/index.js --headless --no-auto-update --log-level info --stdio(http block)/opt/hostedtoolcache/node/22.22.0/x64/bin/node node /home/REDACTED/work/copilot-sdk/copilot-sdk/nodejs/node_modules/@github/copilot/index.js --headless --no-auto-update --log-level info --stdio /usr/local/.ghcu--64(http block)/opt/hostedtoolcache/node/22.22.0/x64/bin/node node /home/REDACTED/work/copilot-sdk/copilot-sdk/nodejs/node_modules/@github/copilot/index.js --headless --no-auto-update --log-level info --stdio x64/pkg/tool/lin--64(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.